<html>
<head><meta charset="utf-8"><title>Completion refactoring · t-compiler/rust-analyzer · Zulip Chat Archive</title></head>
<h2>Stream: <a href="https://rust-lang.github.io/zulip_archive/stream/185405-t-compiler/rust-analyzer/index.html">t-compiler/rust-analyzer</a></h2>
<h3>Topic: <a href="https://rust-lang.github.io/zulip_archive/stream/185405-t-compiler/rust-analyzer/topic/Completion.20refactoring.html">Completion refactoring</a></h3>

<hr>

<base href="https://rust-lang.zulipchat.com">

<head><link href="https://rust-lang.github.io/zulip_archive/style.css" rel="stylesheet"></head>

<a name="213647413"></a>
<h4><a href="https://rust-lang.zulipchat.com#narrow/stream/185405-t-compiler/rust-analyzer/topic/Completion%20refactoring/near/213647413" class="zl"><img src="https://rust-lang.github.io/zulip_archive/assets/img/zulip.svg" alt="view this post on Zulip" style="width:20px;height:20px;"></a> Kirill Bulatov <a href="https://rust-lang.github.io/zulip_archive/stream/185405-t-compiler/rust-analyzer/topic/Completion.20refactoring.html#213647413">(Oct 17 2020 at 11:22)</a>:</h4>
<p>I have a recuring feeling that something is odd with the way we try to apply the completions and the recent related MR <a href="https://github.com/rust-analyzer/rust-analyzer/pull/6262">https://github.com/rust-analyzer/rust-analyzer/pull/6262</a> had given me this feeling again, so I want to discuss it a bit and hear others' thoughts.</p>
<p>Currently, we deal with the completions relatively simple: <a href="https://github.com/rust-analyzer/rust-analyzer/blob/master/crates/ide/src/completion.rs#L120">https://github.com/rust-analyzer/rust-analyzer/blob/master/crates/ide/src/completion.rs#L120</a><br>
First, we create two objects: a <code>CompletionContext</code> with all the data the completions need and a <code>mut Completions</code> object to insert the completion options into.<br>
Then both objects get passed to each completion, that contains an internal logic for everything, related to a certain completion.</p>
<p>In general, it works fine before we start to add exceptions and cover the corner cases.<br>
What's more interesting, some corner cases cases are still not covered, and when another appears others' code bloats.</p>
<p>Example: attribute text completion. Before that, nobody cared what was completed after <code>#[</code> and we did propose to complete a lot there, even though it was not correct.<br>
When I've added this completion, I've had to do the following:</p>
<ul>
<li>add another field into the <code>CompletionContext</code>, showing if we're in somewhere right after <code>#[</code></li>
<li>add a check in the code of every completion that is inappropriately popping up after <code>#[</code> now</li>
<li>write some code into the attribute completion module, also checking for some more corner cases<br>
Later, I've been adding the <code>mod</code> completion which followed the same path: new field in the context, new wave of extra checks, finally some new code.<br>
Another example is the PR that I've mentioned in the first sentence: it disables completion for some cases completely and manages to perform the checks before running the completions, but still has to add two more fields into the <code>CompletionContext</code>.</li>
</ul>
<p>IMO, this all scales pretty bad: <code>CompletionContext</code> now has 48 fields, the majority of them are used either in a single particular completion module (to derive the completion data out of it) or in every module, but just to check that it's appropriate to complete here. Also, some completion modules now have 5 such checks and will get some more, most probably.</p>
<p>My proposal: add a trait similar to</p>
<div class="codehilite"><pre><span></span><code>trait Completion {
    type CompletionData;
    fn derive_completion_data(ctx: &amp;CompletionContext) -&gt; Option&lt;Self::CompletionData&gt;;
    fn complete(acc: &amp;mut Completions, ctx: &amp;CompletionContext) -&gt; Option&lt;()&gt;;
}
</code></pre></div>


<p>so that we can move the majority of the <code>CompletionContext</code> fields into <code>CompletionData</code> + remove all the checks there too.<br>
This way we can allow no completion-specific data in the context: let it all to be derived during the <code>CompletionData</code> and leave only the shared things like <code>Semantics</code>, <code>RootDatabase</code> and similar.</p>
<p>This approach does not seem to solve the checks issue entirely, we still have to check for some extra things inside <code>derive_completion_data</code>, but IMO a better api can solve this: since the <code>derive_completion_data</code> method is focused on getting the data, it should be possible.</p>
<p>Any thoughts or ideas on the topic? (maybe <span class="user-mention" data-user-id="133169">@matklad</span>  ?)</p>



<a name="213647569"></a>
<h4><a href="https://rust-lang.zulipchat.com#narrow/stream/185405-t-compiler/rust-analyzer/topic/Completion%20refactoring/near/213647569" class="zl"><img src="https://rust-lang.github.io/zulip_archive/assets/img/zulip.svg" alt="view this post on Zulip" style="width:20px;height:20px;"></a> Kirill Bulatov <a href="https://rust-lang.github.io/zulip_archive/stream/185405-t-compiler/rust-analyzer/topic/Completion.20refactoring.html#213647569">(Oct 17 2020 at 11:26)</a>:</h4>
<p>For a context, I'm slowly working on the automatic <code>use</code> insertion during the completion and this particular task is very good at showing the places we have to restrict from having unnecessary completions:</p>
<p><a href="/user_uploads/4715/05IvSD5XEalmJLwQ0g14dj5M/Screenshot-2020-10-17-at-14.23.40.png">Screenshot-2020-10-17-at-14.23.40.png</a> </p>
<div class="message_inline_image"><a href="/user_uploads/4715/05IvSD5XEalmJLwQ0g14dj5M/Screenshot-2020-10-17-at-14.23.40.png" title="Screenshot-2020-10-17-at-14.23.40.png"><img src="/user_uploads/4715/05IvSD5XEalmJLwQ0g14dj5M/Screenshot-2020-10-17-at-14.23.40.png"></a></div><p><a href="/user_uploads/4715/QnJmWccjNqITEFpdaMindsp3/Screenshot-2020-10-17-at-14.23.59.png">Screenshot-2020-10-17-at-14.23.59.png</a> </p>
<div class="message_inline_image"><a href="/user_uploads/4715/QnJmWccjNqITEFpdaMindsp3/Screenshot-2020-10-17-at-14.23.59.png" title="Screenshot-2020-10-17-at-14.23.59.png"><img src="/user_uploads/4715/QnJmWccjNqITEFpdaMindsp3/Screenshot-2020-10-17-at-14.23.59.png"></a></div><p>But with the current approach this means plenty of new fields added into the <code>CompletionContext</code> + plenty of extra checks to add into the existing completions, if we want to make it proper.</p>



<a name="213648705"></a>
<h4><a href="https://rust-lang.zulipchat.com#narrow/stream/185405-t-compiler/rust-analyzer/topic/Completion%20refactoring/near/213648705" class="zl"><img src="https://rust-lang.github.io/zulip_archive/assets/img/zulip.svg" alt="view this post on Zulip" style="width:20px;height:20px;"></a> Florian Diebold <a href="https://rust-lang.github.io/zulip_archive/stream/185405-t-compiler/rust-analyzer/topic/Completion.20refactoring.html#213648705">(Oct 17 2020 at 12:00)</a>:</h4>
<ol>
<li>the <code>no_completion_required</code> flag added in that PR doesn't seem like a good idea to me. IMO the particular completions should be more careful about when they show up instead. For example if we apparently complete unqualified paths in the <code>for i i&lt;|&gt;</code> case, that seems to me like a bug in the code setting <code>is_trivial_path</code> or <code>is_pat_binding_or_const</code>, since neither of these should be true in that case.</li>
<li>IMO the many fields of <code>CompletionContext</code> aren't necessarily a problem, though some of them could maybe be combined into enums instead. I don't really see how the trait would work; the <code>CompletionData</code> has to be kept somewhere, doesn't it? If this is just about grouping the data, we could just use structs and functions without a trait. I'm not that convinced that it would make it clearer though.</li>
<li>why is this more of a problem with automatic <code>use</code> insertion than before? shouldn't autoimport just add to the existing dot and unqualified path completions? E.g. why would we suddenly complete a static function like <code>handle_alloc_error</code> on a dot receiver?</li>
</ol>



<a name="213649052"></a>
<h4><a href="https://rust-lang.zulipchat.com#narrow/stream/185405-t-compiler/rust-analyzer/topic/Completion%20refactoring/near/213649052" class="zl"><img src="https://rust-lang.github.io/zulip_archive/assets/img/zulip.svg" alt="view this post on Zulip" style="width:20px;height:20px;"></a> Florian Diebold <a href="https://rust-lang.github.io/zulip_archive/stream/185405-t-compiler/rust-analyzer/topic/Completion.20refactoring.html#213649052">(Oct 17 2020 at 12:10)</a>:</h4>
<blockquote>
<p>some of them could maybe be combined into enums instead</p>
</blockquote>
<p>e.g. I could imagine having just one field that says what kind of identifier we are on (unqualified path, segment of a qualified path, method/field name, item name, attribute, etc.)</p>



<a name="213650367"></a>
<h4><a href="https://rust-lang.zulipchat.com#narrow/stream/185405-t-compiler/rust-analyzer/topic/Completion%20refactoring/near/213650367" class="zl"><img src="https://rust-lang.github.io/zulip_archive/assets/img/zulip.svg" alt="view this post on Zulip" style="width:20px;height:20px;"></a> popzxc <a href="https://rust-lang.github.io/zulip_archive/stream/185405-t-compiler/rust-analyzer/topic/Completion.20refactoring.html#213650367">(Oct 17 2020 at 12:49)</a>:</h4>
<p>I have no strong opinion on the design decisions of the rust analyzer, so my thoughts are just thoughts of the project user who sporadically submits pull request to add a required feature / fix a problem I personally met.</p>
<p>Regarding the <code>no_completion_required</code> flag I partially agree that it isn't a correct way to deal with the problem. The correct way is to not provide a suggestion in these cases at all.<br>
However, the correct way has two significant issues:</p>
<ol>
<li>In the current state, I found the infrastructure to be centered on the "what can we suggest to user?" rather than "what should we not?". Suggestions are spawned from different places and in the current form implementing the same fix would require checking the same thing in the multiple places, which is much less convenient and would have resulted in a bigger amount of code.</li>
<li>The problem already exists and affects the UX. Discussing and implementing a fully correct way to deal with the issue could have resulted in longer time to fix the problem.</li>
</ol>
<p>What I've tried to do in my pull request is to achieve the required functionality without integrating the solution deeply into the code base. It can be quickly removed as soon as proper solution is found, and it should work fine <em>for the nearest future</em>.</p>



<a name="213650372"></a>
<h4><a href="https://rust-lang.zulipchat.com#narrow/stream/185405-t-compiler/rust-analyzer/topic/Completion%20refactoring/near/213650372" class="zl"><img src="https://rust-lang.github.io/zulip_archive/assets/img/zulip.svg" alt="view this post on Zulip" style="width:20px;height:20px;"></a> popzxc <a href="https://rust-lang.github.io/zulip_archive/stream/185405-t-compiler/rust-analyzer/topic/Completion.20refactoring.html#213650372">(Oct 17 2020 at 12:49)</a>:</h4>
<p>Regarding the problem itself, my vision is that the current approach indeed lacks a few abstractions. I think currently <code>CompletionContext</code> attempts to do two jobs at once: understand which types of hints are applicable in the current position, and which hints can be produced. It could be a simpler problem, if spawning suggestions was done in several subsequent steps, like:</p>
<ol>
<li>Check whether types of completions are applicable in the current context (types / keywords / variables / etc).</li>
<li>Create a list of completions for the required types only.</li>
<li>Provide an individual score for each suggestion, depending on both completion itself and context in which completion is going to be applied. First point estimates how close suggestion is to what user typed so far (which later may evolve in a sort of fuzzy search algorithm), while the second point thinks how good the suggestion would work in the code, being placed there.</li>
</ol>
<p>With each step isolated, and it probably would be less error-prone and easier to scale, as the amount of data required for each individual step would be lesser.</p>



<a name="213650424"></a>
<h4><a href="https://rust-lang.zulipchat.com#narrow/stream/185405-t-compiler/rust-analyzer/topic/Completion%20refactoring/near/213650424" class="zl"><img src="https://rust-lang.github.io/zulip_archive/assets/img/zulip.svg" alt="view this post on Zulip" style="width:20px;height:20px;"></a> Kirill Bulatov <a href="https://rust-lang.github.io/zulip_archive/stream/185405-t-compiler/rust-analyzer/topic/Completion.20refactoring.html#213650424">(Oct 17 2020 at 12:50)</a>:</h4>
<p>// My answer to initial Florian's reply, looks like it took me too long to type the reply :)</p>
<ol>
<li>I will look for the bug later, thank you for checking.</li>
<li>The trait is an attempt to make the addition of the new completion module simpler, closer to the way we do it for assists and diagnostics: it takes to add a new module and a one-line change to register the module in the parent module.<br>
For completions it takes a new module, sometimes changes to other modules, because you see that they complete in that place too; usually changes in the completion context and lastly the registering one-liner.</li>
</ol>
<p>Writing the new assist also does not involve scrolling 40-something fields in search of a proper combination to extract the data/check the applicability.</p>
<p>Feels a bit more cumbersome, so I look for ways to improve the situation with a common trait, similar to our diagnostics approach.</p>
<p>The data stays in the actual types for CompletionData, the struct can be defined in the same new module.</p>
<ol start="3">
<li>Maybe a bad example, my bad.</li>
</ol>



<a name="213671718"></a>
<h4><a href="https://rust-lang.zulipchat.com#narrow/stream/185405-t-compiler/rust-analyzer/topic/Completion%20refactoring/near/213671718" class="zl"><img src="https://rust-lang.github.io/zulip_archive/assets/img/zulip.svg" alt="view this post on Zulip" style="width:20px;height:20px;"></a> matklad <a href="https://rust-lang.github.io/zulip_archive/stream/185405-t-compiler/rust-analyzer/topic/Completion.20refactoring.html#213671718">(Oct 17 2020 at 21:32)</a>:</h4>
<p>Haven't caught up with the discussion, but I am noticing that completion is slow to compile if you add debut prins to type infernece and such</p>



<a name="213671726"></a>
<h4><a href="https://rust-lang.zulipchat.com#narrow/stream/185405-t-compiler/rust-analyzer/topic/Completion%20refactoring/near/213671726" class="zl"><img src="https://rust-lang.github.io/zulip_archive/assets/img/zulip.svg" alt="view this post on Zulip" style="width:20px;height:20px;"></a> matklad <a href="https://rust-lang.github.io/zulip_archive/stream/185405-t-compiler/rust-analyzer/topic/Completion.20refactoring.html#213671726">(Oct 17 2020 at 21:32)</a>:</h4>
<p>in particular, it uses compiles assists and ssr</p>



<a name="213671729"></a>
<h4><a href="https://rust-lang.zulipchat.com#narrow/stream/185405-t-compiler/rust-analyzer/topic/Completion%20refactoring/near/213671729" class="zl"><img src="https://rust-lang.github.io/zulip_archive/assets/img/zulip.svg" alt="view this post on Zulip" style="width:20px;height:20px;"></a> matklad <a href="https://rust-lang.github.io/zulip_archive/stream/185405-t-compiler/rust-analyzer/topic/Completion.20refactoring.html#213671729">(Oct 17 2020 at 21:33)</a>:</h4>
<p>perhaps its time to move completion to a separate crate</p>



<a name="213694164"></a>
<h4><a href="https://rust-lang.zulipchat.com#narrow/stream/185405-t-compiler/rust-analyzer/topic/Completion%20refactoring/near/213694164" class="zl"><img src="https://rust-lang.github.io/zulip_archive/assets/img/zulip.svg" alt="view this post on Zulip" style="width:20px;height:20px;"></a> popzxc <a href="https://rust-lang.github.io/zulip_archive/stream/185405-t-compiler/rust-analyzer/topic/Completion.20refactoring.html#213694164">(Oct 18 2020 at 07:48)</a>:</h4>
<p>I am potentially interested in working on the completions module. If there will be agreement on the proposed design, I can nominate myself to bring it to the life.</p>



<a name="213695018"></a>
<h4><a href="https://rust-lang.zulipchat.com#narrow/stream/185405-t-compiler/rust-analyzer/topic/Completion%20refactoring/near/213695018" class="zl"><img src="https://rust-lang.github.io/zulip_archive/assets/img/zulip.svg" alt="view this post on Zulip" style="width:20px;height:20px;"></a> matklad <a href="https://rust-lang.github.io/zulip_archive/stream/185405-t-compiler/rust-analyzer/topic/Completion.20refactoring.html#213695018">(Oct 18 2020 at 08:13)</a>:</h4>
<p>Cool! Still hanvn't caught up in the discussion, but I think, regardless of what design we choose, it would be better to split completions into a crate before that</p>



<a name="213695072"></a>
<h4><a href="https://rust-lang.zulipchat.com#narrow/stream/185405-t-compiler/rust-analyzer/topic/Completion%20refactoring/near/213695072" class="zl"><img src="https://rust-lang.github.io/zulip_archive/assets/img/zulip.svg" alt="view this post on Zulip" style="width:20px;height:20px;"></a> matklad <a href="https://rust-lang.github.io/zulip_archive/stream/185405-t-compiler/rust-analyzer/topic/Completion.20refactoring.html#213695072">(Oct 18 2020 at 08:14)</a>:</h4>
<p>That is, there's some obvious work to be done even before having consensus on design :)</p>



<a name="213699663"></a>
<h4><a href="https://rust-lang.zulipchat.com#narrow/stream/185405-t-compiler/rust-analyzer/topic/Completion%20refactoring/near/213699663" class="zl"><img src="https://rust-lang.github.io/zulip_archive/assets/img/zulip.svg" alt="view this post on Zulip" style="width:20px;height:20px;"></a> popzxc <a href="https://rust-lang.github.io/zulip_archive/stream/185405-t-compiler/rust-analyzer/topic/Completion.20refactoring.html#213699663">(Oct 18 2020 at 10:13)</a>:</h4>
<p>Done, I guess. <a href="https://github.com/rust-analyzer/rust-analyzer/pull/6276">https://github.com/rust-analyzer/rust-analyzer/pull/6276</a></p>



<a name="213706160"></a>
<h4><a href="https://rust-lang.zulipchat.com#narrow/stream/185405-t-compiler/rust-analyzer/topic/Completion%20refactoring/near/213706160" class="zl"><img src="https://rust-lang.github.io/zulip_archive/assets/img/zulip.svg" alt="view this post on Zulip" style="width:20px;height:20px;"></a> matklad <a href="https://rust-lang.github.io/zulip_archive/stream/185405-t-compiler/rust-analyzer/topic/Completion.20refactoring.html#213706160">(Oct 18 2020 at 13:20)</a>:</h4>
<p>Finally got the time to catch up with the discussion :0)</p>
<p>100% agree that current architecture of completions is a bit of a mess. I also think that the current <em>state</em> of completion might be a mess. Up until relatively recently, completions from rust-analyzer were a sort of fragile miracle, so we tried to keep as many variants as possible. Today, when coverage of the base language is pretty thorough, it makes sense to focus on better filtering and sorting -- not providing erroneous completions, putting good stuff on top. This might require architectural rethinking.</p>
<p>What I like about current architecture is how all completion variants are independent. I like how we "compose" them together with simple function calls here:</p>
<p><a href="https://github.com/rust-analyzer/rust-analyzer/blob/886cfd68212bb0b4487d6a822476c350a6eb114f/crates/completion/src/lib.rs#L121-L135">https://github.com/rust-analyzer/rust-analyzer/blob/886cfd68212bb0b4487d6a822476c350a6eb114f/crates/completion/src/lib.rs#L121-L135</a></p>
<p>Each specific completion kind also doesn't look terrible to me:</p>
<p><a href="https://github.com/rust-analyzer/rust-analyzer/blob/886cfd68212bb0b4487d6a822476c350a6eb114f/crates/completion/src/complete_unqualified_path.rs#L9-L40">https://github.com/rust-analyzer/rust-analyzer/blob/886cfd68212bb0b4487d6a822476c350a6eb114f/crates/completion/src/complete_unqualified_path.rs#L9-L40</a></p>
<p>(well, attributes and keywords might be exceptions, they do feel messy).</p>
<p>What I don't like is the initial filling of completion context, there's definitely some room for improvement here:</p>
<p><a href="https://github.com/rust-analyzer/rust-analyzer/blob/886cfd68212bb0b4487d6a822476c350a6eb114f/crates/completion/src/completion_context.rs#L350-L499">https://github.com/rust-analyzer/rust-analyzer/blob/886cfd68212bb0b4487d6a822476c350a6eb114f/crates/completion/src/completion_context.rs#L350-L499</a></p>
<p>Finally, I love how low-tech the current infra is -- it's just a bunch of function calls. I think we should strive to preserve low-techness and avoid traits :-) Assists infra might be a good example here. I am pretty sure that whatever overall code shape we design, it would be implementable without traits :-)</p>
<p>One plausible issue with Ctx is that it stores one-shot fields, fields used only in a single completion kind. I think we should move this fields to be just local variables in corresponding completion variants. This probably means that <code>file_with_fake_ident</code> should become a field of <code>Ctx</code> such that completions can inspect it. This seems like a piece of work which can be done in isolation. I wonder how much thinner can we make CompletionCtx after that?</p>
<p>The second issue is that a lot of completion functions have clauses like</p>
<div class="codehilite"><pre><span></span><code>if in_edge_case1 || in_edge_case2 || in_edge_case3 {
   return None
}
</code></pre></div>


<p>Intuitively, such cutting off of cases that <em>won't</em> work seems fragile. I'd much rather prefer to see</p>
<div class="codehilite"><pre><span></span><code>if !(in_applicable_case) {
   return None
}
</code></pre></div>


<p>Ie, instead of listing all of the cases that won't work, we list only specific case that will work. It's worth investigating why this approach doesn't work well. I have a hypothesis that this is because our parser does too good error recovery.</p>
<p>For example, for <code>for _ intelliJRulezzz</code>, we get a tree which looks like this</p>
<div class="codehilite"><pre><span></span><code>FOR_EXPR
  PLACEHOLDER_PAT &quot;_&quot;
  PATH_EXPR
    IDENT         &quot;intelliJRulezzzz&quot;
</code></pre></div>


<p>Kotlin's parser gets this instead:</p>
<div class="codehilite"><pre><span></span><code>FOR_EXPR
  PLACEHOLDER_PAT &quot;_&quot;
  SYNTAX_ERROR
    IDENT         &quot;intelliJRulezzzz&quot;
</code></pre></div>


<p>We should change the parser to be more like IntelliJ. I don't think there's a general rule here, but I also feel that there maybe many specific rules, fixing all of which would get rid of most of the bad ifs.</p>
<p>So, action items would be:</p>
<ul>
<li>Make it <em>possible</em> to get context when doing completion, and not only in <code>Ctx::fill</code></li>
<li>Go over each <code>Ctx</code>'s field, and, if it has unique usage, push it down</li>
<li>Go over each special <code>in_edge_case</code>, find the reason <em>why</em> <code>in_applicable_case</code> won't work there, fix it.</li>
</ul>
<p>These are very tactical small steps, but I'd be curious where the full series of such steps leads to.</p>



<a name="213874839"></a>
<h4><a href="https://rust-lang.zulipchat.com#narrow/stream/185405-t-compiler/rust-analyzer/topic/Completion%20refactoring/near/213874839" class="zl"><img src="https://rust-lang.github.io/zulip_archive/assets/img/zulip.svg" alt="view this post on Zulip" style="width:20px;height:20px;"></a> popzxc <a href="https://rust-lang.github.io/zulip_archive/stream/185405-t-compiler/rust-analyzer/topic/Completion.20refactoring.html#213874839">(Oct 20 2020 at 04:39)</a>:</h4>
<p><span class="user-mention" data-user-id="133169">@matklad</span>  OK, now with the completions crate moved out, I guess the next step is what you mentioned in the comment: <a href="https://github.com/rust-analyzer/rust-analyzer/pull/6276#issuecomment-711148963">https://github.com/rust-analyzer/rust-analyzer/pull/6276#issuecomment-711148963</a></p>
<p>I'm not sure if I'll have the time to work on it till Saturday, but hopefully will do it this weekend.</p>



<a name="213892217"></a>
<h4><a href="https://rust-lang.zulipchat.com#narrow/stream/185405-t-compiler/rust-analyzer/topic/Completion%20refactoring/near/213892217" class="zl"><img src="https://rust-lang.github.io/zulip_archive/assets/img/zulip.svg" alt="view this post on Zulip" style="width:20px;height:20px;"></a> matklad <a href="https://rust-lang.github.io/zulip_archive/stream/185405-t-compiler/rust-analyzer/topic/Completion.20refactoring.html#213892217">(Oct 20 2020 at 09:09)</a>:</h4>
<p>Yup, that would be a good point! If you feel like refactoring the physical architecture more, shortening that compilation critical path would be useful too!</p>



<a name="229245134"></a>
<h4><a href="https://rust-lang.zulipchat.com#narrow/stream/185405-t-compiler/rust-analyzer/topic/Completion%20refactoring/near/229245134" class="zl"><img src="https://rust-lang.github.io/zulip_archive/assets/img/zulip.svg" alt="view this post on Zulip" style="width:20px;height:20px;"></a> Josh Mcguigan <a href="https://rust-lang.github.io/zulip_archive/stream/185405-t-compiler/rust-analyzer/topic/Completion.20refactoring.html#229245134">(Mar 08 2021 at 04:15)</a>:</h4>
<p>I found this thread when trying to understand some things about the structure of the completions code. Specifically I was working on adding scoring for function calls. </p>
<p><a href="https://github.com/JoshMcguigan/rust-analyzer/compare/master...JoshMcguigan:complete-fn-score">https://github.com/JoshMcguigan/rust-analyzer/compare/master...JoshMcguigan:complete-fn-score</a></p>
<p>I think this additional scoring would be useful, as it ensure functions with the desired return type are sorted to the top of the completions. But I found what feels like the cases being discussed above regarding the fragility and over-connectedness of the various completion types. Specifically, I had to add an <code>active_name</code> method, even though there is already an <code>active_name_and_type</code> method, because sometimes the <code>active_name_and_type</code> method returns <code>None</code> in places that my completion cares about. So the completion I was working on uses this new <code>active_name</code> method and <code>self.completion.expected_type</code>. </p>
<p>I've spent some time trying to understand why the type stored in the <code>expected_type</code> field on <code>CompletionContext</code> would be different than the type returned by <code>active_name_and_type</code>, but I haven't been able to figure out if that difference is intentional. It seems like this is a case of some completions want slightly different information than others.</p>
<p>Just to clear up my own understanding, should it be possible for all completions to work against a single <code>expected_type</code>? Does <code>active_type</code> mean something different than <code>expected_type</code>?</p>



<a name="229546493"></a>
<h4><a href="https://rust-lang.zulipchat.com#narrow/stream/185405-t-compiler/rust-analyzer/topic/Completion%20refactoring/near/229546493" class="zl"><img src="https://rust-lang.github.io/zulip_archive/assets/img/zulip.svg" alt="view this post on Zulip" style="width:20px;height:20px;"></a> matklad <a href="https://rust-lang.github.io/zulip_archive/stream/185405-t-compiler/rust-analyzer/topic/Completion.20refactoring.html#229546493">(Mar 09 2021 at 20:21)</a>:</h4>
<p><span class="user-mention" data-user-id="275435">@Josh Mcguigan</span> urgh, yeah, you are correct, <code>expected_name_and_type</code> doesn't make sense, we should just use the one recorded on the ctx I think</p>



<hr><p>Last updated: Aug 07 2021 at 22:04 UTC</p>
</html>